[CrashReporter] Enable InProc CrashReporter for crashing GC threads - #131821
[CrashReporter] Enable InProc CrashReporter for crashing GC threads#131821mdh1418 wants to merge 1 commit into
Conversation
Reuse an existing suspension only when the crashing thread owns it, and allow workstation background GC to perform reporter-owned suspend and resume while preserving Server GC deadlock safeguards. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: aa976a9e-b875-4524-82e1-1485d03d14fa
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR updates CoreCLR’s in-proc crash-report stack walker to allow managed thread enumeration in additional GC/suspension scenarios by tracking whether a runtime suspension is (a) unavailable, (b) already established and owned by the crashing thread, or (c) created by the crash reporter.
Changes:
- Introduces
CrashReportSuspensionOwnershipto distinguish between an unusable suspension, a usable existing suspension, and a reporter-created suspension. - Reuses an existing completed suspension only when the crashing thread owns the ThreadStore lock; otherwise avoids enumerating other managed threads.
- Resumes the runtime only when the crash reporter itself performed the suspension.
| // Do not wait for the ThreadStore lock while another suspension is starting or | ||
| // ending. In particular, a Server GC coordinator may hold the lock while waiting | ||
| // for a crashing parallel worker at a GC join. A fatal error already recorded on | ||
| // a GC thread also means the interrupted GC may never complete. | ||
| // |
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
| { | ||
| return crashThreadOwnsSuspension | ||
| ? CrashReportSuspensionOwnership::Existing | ||
| : CrashReportSuspensionOwnership::Unavailable; |
There was a problem hiding this comment.
If I am reading this correctly, we will take CrashReportSuspensionOwnership::Unavailable; path when encounter a crash on a server GC thread. Is it the behavior we want?
There was a problem hiding this comment.
The scenario I was imagining is in Server GC, there is a dedicated heap 0 thread that coordinates suspension, and the crash occurs in a Parallel Server GC worker. I was thinking that since the heap 0 thread controls SuspendEE/RestartEE that it wouldn't be safe to stackwalk other threads if the heap 0 thread can resume independently.
If the crashing thread is the one that owns Suspension, then we are able to walk suspended threads callstacks given it would be the one to RestartEE.
Does that sound right?
There was a problem hiding this comment.
In Server GC, the thread that decides to trigger the GC suspends the runtime. Once the runtime is suspended, it signals to server GC threads to do the GC and waits for them to report back that the GC is done. If you get a crash on server GC thread, it should be safe to assume that the runtime is suspended.
Try to run this program that corrupts GC heap with server GC:
using System.Runtime.CompilerServices;
var o = new Test();
unsafe
{
fixed (int* p = &o.x)
*(p-1) = 0x12345678;
}
GC.Collect();
GC.KeepAlive(o);
class Test
{
public int x;
}
When the GC crashes, you should see the crash on GC thread. The main thread should be waiting at:
coreclr!SVR::gc_heap::wait_for_gc_done+0x52 [D:\a\_work\1\s\src\runtime\src\coreclr\gc\gc.cpp @ 15055]
coreclr!SVR::GCHeap::GarbageCollectGeneration+0xf8 [D:\a\_work\1\s\src\runtime\src\coreclr\gc\gc.cpp @ 51759]
coreclr!SVR::GCHeap::GarbageCollect+0xfe [D:\a\_work\1\s\src\runtime\src\coreclr\gc\gc.cpp @ 50890]
coreclr!GCInterface_Collect+0x66 [D:\a\_work\1\s\src\runtime\src\coreclr\vm\comutilnative.cpp @ 851]
System_Private_CoreLib!System.GC.Collect+0x64 [/_/src/runtime/src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs @ 177]
repro!Program.<Main>$+0x90 [C:\repro\Program.cs @ 9]
Previously, the InProc CrashReporter skipped suspending and enumerating other managed threads during any GC scenario. However, in select GC scenarios, the thread running the reporter either owns the existing thread suspension or can safely establish its own suspension and walk managed call stacks.
This change tracks suspension ownership so the reporter can: